Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modification by filter #2186

Merged
merged 45 commits into from
Oct 3, 2024
Merged

Modification by filter #2186

merged 45 commits into from
Oct 3, 2024

Conversation

thangqp
Copy link
Contributor

@thangqp thangqp commented Jul 22, 2024 •
edited
Loading

@thangqp thangqp mentioned this pull request Jul 23, 2024
Merged
@thangqp thangqp closed this Jul 24, 2024
@thangqp thangqp deleted the modification_by_filter branch July 24, 2024 16:27
@thangqp thangqp restored the modification_by_filter branch July 24, 2024 17:29
@thangqp thangqp reopened this Jul 24, 2024
thangqp and others added 22 commits July 25, 2024 17:20
@thangqp
using rhf inputs for value editor
ffce003
@thangqp
Merge branch 'main' into modification_by_filter
d434265
@thangqp
rename FLOAT dataType to NUMBER dataType
ae9dff2
@thangqp
Merge branch 'main' into modification_by_filter
c6d2d65
@thangqp
Merge branch 'main' into modification_by_filter
fa91854
@thangqp
rename NUMBER dataType to DOUBLE dataType
e4f86b9
@thangqp
send dataType to backend
687bee8
@thangqp
Merge from main
f70f8f0
@thangqp
Merge branch 'main' into modification_by_filter
6fdf5a8
@thangqp
format prettier
812439f
@thangqp
rename dto field
9d3324d
@thangqp
add boolean and enum fields
1a280b3
@thangqp
support property for all equipment types
5f8e619
@thangqp
dialogue in TSX
1c7c2c3
@thangqp
renaming
4a2394b
@thangqp
typing problem with rhf
8a1f622
@thangqp
reset value when data type changed
0ef88c8
@thangqp
set default value for boolean type
e261338
@thangqp
remove usage of searchTree of common ui
488270a
@thangqp
Merge branch 'main' into modification_by_filter
d8df6e7
@thangqp
Merge branch 'main' into modification_by_filter
bde8046
@thangqp
using localCompare when sort
f2ce391
Mathieu-Deharbe
Mathieu-Deharbe requested changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be because I am using the modification_by_filter_no_migration_data branch, just let me know. But I am losing a value if I close and reopen the field :

perte de valeur

If I apply the network modification this works fine so the value is saved in the database.

But in the log, the new value (12) is written nowhere 👍

image

Not a bug but I think that would be a good improvement to add it.

=> OK DONE

@thangqp
Copy link
Contributor Author

thangqp commented Sep 12, 2024 •
edited
Loading

This may be because I am using the modification_by_filter_no_migration_data branch, just let me know. But I am losing a value if I close and reopen the field :

perte de valeur perte de valeur

If I apply the network modification this works fine so the value is saved in the database.

But in the log, the new value (12) is written nowhere 👍

image

Not a bug but I think that would be a good improvement to add it.

The first issue, I will verify what happens

The second issue, we can show severity TRACE to show the transition of values

image

ghazwarhili
ghazwarhili reviewed Sep 13, 2024
View reviewed changes
...ponents/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-utils.ts Outdated
})
)
.required()
.min(1, 'FieldIsRequired'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no translation for FieldIsRequired ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translation looks fine to me :

"FieldIsRequired": "This field is required",
"FieldIsRequired": "Ce champ doit être renseigné",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YupRequired is a newer key, I think should switch to the newer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE

ghazwarhili
ghazwarhili reviewed Sep 13, 2024
View reviewed changes
...ponents/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-form.tsx Outdated
interface AssignmentFormProps {
name: string;
index: number;
predefinedProperties: any;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type of predefinedProperties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE

Mathieu-Deharbe
Mathieu-Deharbe requested changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The '=' icon required in the PO ticket is missing (but exists in the modification by formula).

image

OK DONE

...nts/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-constants.ts Outdated Show resolved Hide resolved
...nts/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-constants.ts Outdated Show resolved Hide resolved
...nts/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-constants.ts Outdated Show resolved Hide resolved
...ponents/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-form.tsx Outdated Show resolved Hide resolved
...ponents/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-form.tsx Outdated Show resolved Hide resolved
Mathieu-Deharbe
Mathieu-Deharbe reviewed Sep 17, 2024
View reviewed changes
...ponents/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-utils.ts Outdated
})
)
.required()
.min(1, 'FieldIsRequired'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translation looks fine to me :

"FieldIsRequired": "This field is required",
"FieldIsRequired": "Ce champ doit être renseigné",

...nts/dialogs/network-modifications/by-filter/by-assignment/assignment/assignment-constants.ts Outdated Show resolved Hide resolved
src/services/study/network-modifications.js Outdated Show resolved Hide resolved
ghazwarhili
ghazwarhili approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@ghazwarhili ghazwarhili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code => OK

.../dialogs/network-modifications/by-filter/by-assignment/modification-by-assignment-dialog.tsx Outdated Show resolved Hide resolved
@thangqp
remark
9b6db2a 
Signed-off-by: Thang PHAM <phamthang37@gmail.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 3, 2024

Quality Gate Passed Quality Gate passed

Issues
13 New issues
0 Accepted issues

Measures
0 Security Hotspots
7.8% Coverage on New Code
1.5% Duplication on New Code

See analysis details on SonarCloud

@thangqp thangqp merged commit 3ea6d01 into main Oct 3, 2024
4 checks passed
@thangqp thangqp deleted the modification_by_filter branch October 3, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghazwarhili ghazwarhili ghazwarhili approved these changes

@Mathieu-Deharbe Mathieu-Deharbe Mathieu-Deharbe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thangqp @ghazwarhili @Mathieu-Deharbe